Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate XRT Include Directory in CMake target #8027

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wilderfield
Copy link
Collaborator

Problem solved by the commit

When foreign CMake projects link against XRT::xrt_coreutil
The target's property INTERFACE_INCLUDE_DIRECTORIES is empty.
Today our customers resolve this by doing

add_library(myLib myLib.cpp)
target_include_directories(myLib ${XRT_INCLUDE_DIRS}) # Bring in all the includes
target_link_libraries(myLib XRT::xrt_coreutil)

This really shouldn't be necessary, because this library is unusable without header files.
We can allow the include directory to propagate.

This will fix issues where another project creates an interface library that links XRT.

For instance here is a hack I'm having to do:

# XRT library target is unaware of its associated include files
 # Get the existing interface include directories
 get_target_property(include_dirs ryzenai::qlinear_2 INTERFACE_INCLUDE_DIRECTORIES)

 # Append the new include directory
 list(APPEND include_dirs "${XRT_INCLUDE_DIRS}")

 # Set the modified include directories back on the target
 set_target_properties(ryzenai::qlinear_2 PROPERTIES
     INTERFACE_INCLUDE_DIRECTORIES "${include_dirs}"
 )

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

Discovered while trying to create an interface library that links XRT.

How problem was solved, alternative solutions (if any) and why they were rejected

See code change

Risks (if any) associated the changes in the commit

None

What has been tested and how, request additional testing if necessary

I've done no testing, this is my proposed fix to ensure that the include directory property can propagate to the end user.

Documentation impact (if any)

None

@wilderfield wilderfield requested a review from stsoe as a code owner March 20, 2024 22:41
@gbuildx
Copy link
Collaborator

gbuildx commented Mar 20, 2024

Can one of the admins verify this patch?

@gbuildx
Copy link
Collaborator

gbuildx commented Mar 20, 2024

wilderfield - is not a collaborator
Can XRT admins please validate PR

@stsoe
Copy link
Collaborator

stsoe commented Mar 20, 2024

/build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants